Wait for ystore to be started #43
Conversation
pycrdt_websocket/yroom.py
Outdated
| @@ -135,7 +135,7 @@ def on_message(self, value: Callable[[bytes], Awaitable[bool] | bool] | None): | |||
|
|
|||
| async def _broadcast_updates(self): | |||
| if self.ystore is not None and not self.ystore.started.is_set(): | |||
| self._task_group.start_soon(self.ystore.start) | |||
| await self._task_group.start(self.ystore.start) | |||
There was a problem hiding this comment.
_broadcast_updates method is scheduled as start_soon task still. We could not guarantee that this ystore.start method will run before ystore.start method in DocumentRoom initialize room method. A race condition could happen. We might want to capture exception here in case that ystore task group has already been started.
There was a problem hiding this comment.
This PR doesn't specifically targets the issue you are mentioning, it's just to ensure that the YStore has effectively started (which was not guaranteed with start_soon).
That said, you're raising a good point, as starting a YStore could be done concurrently. A solution would be to have a Lock for a YStore that the user could use like this:
async with ystore.start_lock:
if not ystore.started.is_set():
await task_group.start(ystore.start)I pushed 798120b to use this pattern internally in pycrdt-websocket, and this would have to be used in jupyter-collaboration too.
davidbrochart
force-pushed
the
start-ystore
branch
from
9819f6a to
798120b
Compare
No description provided.